feat: implement node runtime region awareness for cdk vended custom resources #30108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
colifran
changed the title
aws-cdk-automation
previously requested changes
May 8, 2024
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
colifran
commented
May 14, 2024
Comment on lines
-218
to
-240
| type: scope.coreInternal | ||
| ? CORE_INTERNAL_CR_PROVIDER.CustomResourceProviderOptions | ||
| : CORE_MODULE.CustomResourceProviderOptions, |
There was a problem hiding this comment.
This is one example of what the ModuleImporter class helps with. We can maintain one module, e.g., CORE_MODULE, defined with all of it's types, expressions, etc. rather than needing to define a duplicate module just based on where the module would be imported from, i.e. core vs. aws-lambda. Instead, when registering an import we can just register the same module with a different fromLocation specified. Now the code looks a lot cleaner and we maintain a single module definition.
colifran
commented
May 14, 2024
Comment on lines
-274
to
-341
| /** | ||
| * External modules that this class depends on. | ||
| */ | ||
| protected abstract readonly externalModules: ExternalModule[]; | ||
|
|
||
| private importExternalModulesInto(scope: HandlerFrameworkModule) { | ||
| for (const module of this.externalModules) { | ||
| if (!scope.hasExternalModule(module)) { | ||
| scope.addExternalModule(module); | ||
| this.importExternalModuleInto(scope, module); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private importExternalModuleInto(scope: HandlerFrameworkModule, module: ExternalModule) { | ||
| switch (module.fqn) { | ||
| case PATH_MODULE.fqn: { | ||
| PATH_MODULE.import(scope, 'path'); | ||
| return; | ||
| } | ||
| case CONSTRUCTS_MODULE.fqn: { | ||
| CONSTRUCTS_MODULE.importSelective(scope, ['Construct']); | ||
| return; | ||
| } | ||
| case CORE_MODULE.fqn: { | ||
| CORE_MODULE.importSelective(scope, [ | ||
| 'Stack', | ||
| 'CustomResourceProviderBase', | ||
| 'CustomResourceProviderOptions', | ||
| ]); | ||
| return; | ||
| } | ||
| case CORE_INTERNAL_CR_PROVIDER.fqn: { | ||
| CORE_INTERNAL_CR_PROVIDER.importSelective(scope, [ | ||
| 'CustomResourceProviderBase', | ||
| 'CustomResourceProviderOptions', | ||
| ]); | ||
| return; | ||
| } | ||
| case CORE_INTERNAL_STACK.fqn: { | ||
| CORE_INTERNAL_STACK.importSelective(scope, ['Stack']); | ||
| return; | ||
| } | ||
| case LAMBDA_MODULE.fqn: { | ||
| LAMBDA_MODULE.import(scope, 'lambda'); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Lots of reduced code here due to ModuleImporter class managing imports
rix0rrr
reviewed
May 14, 2024
There was a problem hiding this comment.
The overall name of the framework was changed from HandlerFramework to ProviderFramework. This is because the framework is being used to code generate custom resource providers. The custom resource providers execute handler code that this framework has the responsibility of bundling, but calling this a HandlerFramework isn't accurate.
Not in fact true, as far as I'm aware. We call it "custom resource handlers" etc, but I'm pretty sure it also contains Lambda Handlers that don't go into custom resources. Yes, 90% of them are custom resources, but not 100%. So calling this all "custom resources" is a mental shortcut.
| /** | ||
| * The latest Lambda NodeJS runtime available in a given region. | ||
| */ | ||
| public static readonly LATEST_NODE_RUNTIME = 'latestNodeRuntime'; |
There was a problem hiding this comment.
I also would expect this version to be used for lambda.Runtime.LATEST_NODEJS
packages/aws-cdk-lib/custom-resources/lib/provider-framework/provider.ts
Outdated
Show resolved
Hide resolved
colifran
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
colifran
changed the title
aws-cdk-automation
added
the
pr/needs-maintainer-review
scanlonp
reviewed
May 30, 2024
TheRealAmazonKendra
force-pushed
the
colifran/region-aware-framework
branch
2 times, most recently
from
8305b3a
to
73379cf
Compare
aws-cdk-automation
previously requested changes
Jun 6, 2024
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
TheRealAmazonKendra
force-pushed
the
colifran/region-aware-framework
branch
from
73379cf
to
df9727c
Compare
Signed-off-by: Francis <colifran@amazon.com>
Signed-off-by: Francis <colifran@amazon.com>
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
1 task
aws-cdk-automation
removed
the
pr/needs-maintainer-review
TheRealAmazonKendra
approved these changes
Jun 11, 2024
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
amazon-codecatalyst bot
pushed a commit
to pahud/aws-cdk
that referenced
this pull request
Jun 12, 2024
…esources (aws#30108) This PR introduces node runtime region awareness into the custom resource provider framework by doing the following: - Adding a `LATEST_NODE_RUNTIME_MAP` fact table used to maintain the latest node runtime available per AWS partition - Introducing `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` functions which determine the latest Lambda node `Runtime` and the latest Lambda node `Runtime` name, respectively - Updating the custom resource provider framework to utilize these two functions when code generating the runtime property for providers using node runtimes This PR can be segmented into the following code changes: - A `LATEST_NODE_RUNTIME_MAP` fact table was added which maintains the latest Lambda node runtime available per AWS partition. - Introduced `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` functions which determine the latest Lambda node `Runtime` and the latest Lambda node `Runtime` name, respectively. - The existing runtime property being code generated via the custom resource provider framework has been altered to now use the appropriate runtime determiner function, i.e., `determineLatestNodeRuntimeName` for `CustomResourceProvider` or `determineLatestNodeRuntime` for `Function` or `SingletonFunction`. Any custom resource provider using a python runtime will not use either of these functions. - To consolidate and manage importing of external modules a `ModuleImporter` class has been created. This class allows external modules to be registered as an import for a target module and prevents duplicate imports for modules that contain multiple framework components. This class also provides the ability to specify different import paths which allows all external modules to be consolidated into a single class rather than having duplicate external modules defined for different import paths. Lastly, this class determines whether or not external modules should be imported selectively or if all targets in the external module should be imported under an alias, i.e., `import { Function } from 'aws-lambda'` vs. `import * as lambda from 'aws-lambda'` - A `CallableExpr` class was created to allow expression proxies to be created from a specified expression name. This allows the new runtime determiner functions and other module specific functions to be called from their specified module and built into a JavaScript object that will mirror the JavaScript operations done to it in an expression tree. Manually tested all individual custom resource handlers for create, update, and delete. I verified that the behavior seen was what was expected based off of the handler code. All impacted integ tests were updated. New unit tests were added to the custom resource handler framework to test that generated code correctly included `determineLatestNodeRuntime` for Lambda based handlers and `determineLatestNodeRuntimeName` for `CfnResource` based handlers. Added new unit tests to test the functionality of `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` for region agnostic stacks, stacks in commercial region, stacks in China regions, stacks in ADC regions, and stacks in GovCloud regions. - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 task
mazyu36
pushed a commit
to mazyu36/aws-cdk
that referenced
this pull request
Jun 22, 2024
…esources (aws#30108) ### Reason for this change This PR introduces node runtime region awareness into the custom resource provider framework by doing the following: - Adding a `LATEST_NODE_RUNTIME_MAP` fact table used to maintain the latest node runtime available per AWS partition - Introducing `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` functions which determine the latest Lambda node `Runtime` and the latest Lambda node `Runtime` name, respectively - Updating the custom resource provider framework to utilize these two functions when code generating the runtime property for providers using node runtimes ### Description of changes This PR can be segmented into the following code changes: - A `LATEST_NODE_RUNTIME_MAP` fact table was added which maintains the latest Lambda node runtime available per AWS partition. - Introduced `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` functions which determine the latest Lambda node `Runtime` and the latest Lambda node `Runtime` name, respectively. - The existing runtime property being code generated via the custom resource provider framework has been altered to now use the appropriate runtime determiner function, i.e., `determineLatestNodeRuntimeName` for `CustomResourceProvider` or `determineLatestNodeRuntime` for `Function` or `SingletonFunction`. Any custom resource provider using a python runtime will not use either of these functions. - To consolidate and manage importing of external modules a `ModuleImporter` class has been created. This class allows external modules to be registered as an import for a target module and prevents duplicate imports for modules that contain multiple framework components. This class also provides the ability to specify different import paths which allows all external modules to be consolidated into a single class rather than having duplicate external modules defined for different import paths. Lastly, this class determines whether or not external modules should be imported selectively or if all targets in the external module should be imported under an alias, i.e., `import { Function } from 'aws-lambda'` vs. `import * as lambda from 'aws-lambda'` - A `CallableExpr` class was created to allow expression proxies to be created from a specified expression name. This allows the new runtime determiner functions and other module specific functions to be called from their specified module and built into a JavaScript object that will mirror the JavaScript operations done to it in an expression tree. ### Description of how you validated changes Manually tested all individual custom resource handlers for create, update, and delete. I verified that the behavior seen was what was expected based off of the handler code. All impacted integ tests were updated. New unit tests were added to the custom resource handler framework to test that generated code correctly included `determineLatestNodeRuntime` for Lambda based handlers and `determineLatestNodeRuntimeName` for `CfnResource` based handlers. Added new unit tests to test the functionality of `determineLatestNodeRuntime` and `determineLatestNodeRuntimeName` for region agnostic stacks, stacks in commercial region, stacks in China regions, stacks in ADC regions, and stacks in GovCloud regions. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
2 tasks
This was referenced Jun 27, 2024
Merged
1 task
This was referenced Jul 11, 2024
This was referenced Jul 12, 2024
1 task
This was referenced Jul 19, 2024
|
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason for this change
This PR introduces node runtime region awareness into the custom resource provider framework by doing the following:
LATEST_NODE_RUNTIME_MAPfact table used to maintain the latest node runtime available per AWS partitiondetermineLatestNodeRuntimeanddetermineLatestNodeRuntimeNamefunctions which determine the latest Lambda nodeRuntimeand the latest Lambda nodeRuntimename, respectivelyDescription of changes
This PR can be segmented into the following code changes:
LATEST_NODE_RUNTIME_MAPfact table was added which maintains the latest Lambda node runtime available per AWS partition.determineLatestNodeRuntimeanddetermineLatestNodeRuntimeNamefunctions which determine the latest Lambda nodeRuntimeand the latest Lambda nodeRuntimename, respectively.determineLatestNodeRuntimeNameforCustomResourceProviderordetermineLatestNodeRuntimeforFunctionorSingletonFunction. Any custom resource provider using a python runtime will not use either of these functions.ModuleImporterclass has been created. This class allows external modules to be registered as an import for a target module and prevents duplicate imports for modules that contain multiple framework components. This class also provides the ability to specify different import paths which allows all external modules to be consolidated into a single class rather than having duplicate external modules defined for different import paths. Lastly, this class determines whether or not external modules should be imported selectively or if all targets in the external module should be imported under an alias, i.e.,import { Function } from 'aws-lambda'vs.import * as lambda from 'aws-lambda'CallableExprclass was created to allow expression proxies to be created from a specified expression name. This allows the new runtime determiner functions and other module specific functions to be called from their specified module and built into a JavaScript object that will mirror the JavaScript operations done to it in an expression tree.Description of how you validated changes
Manually tested all individual custom resource handlers for create, update, and delete. I verified that the behavior seen was what was expected based off of the handler code. All impacted integ tests were updated. New unit tests were added to the custom resource handler framework to test that generated code correctly included
determineLatestNodeRuntimefor Lambda based handlers anddetermineLatestNodeRuntimeNameforCfnResourcebased handlers. Added new unit tests to test the functionality ofdetermineLatestNodeRuntimeanddetermineLatestNodeRuntimeNamefor region agnostic stacks, stacks in commercial region, stacks in China regions, stacks in ADC regions, and stacks in GovCloud regions.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license